YARN-11634. [Addendum] Speed-up TestTimelineClient.#6419
YARN-11634. [Addendum] Speed-up TestTimelineClient.#6419brumi1024 merged 4 commits intoapache:trunkfrom
Conversation
|
@brumi1024 @K0K0V0K In #6371, we introduced a sputbug. I tried to modify the code, can you help review this PR? Thank you very much! |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
brumi1024
left a comment
There was a problem hiding this comment.
Thanks @slfan1989 for the patch. I have one small comment.
| public HttpURLConnection configure(HttpURLConnection conn) | ||
| throws IOException { | ||
| setTimeouts(conn, DEFAULT_SOCKET_TIMEOUT); | ||
| setTimeouts(conn, 60_000); |
There was a problem hiding this comment.
Nit: the behaviour changed a bet, if someone simply edits the socketTimeout variable, this won't be changed with it. Shouldn't the socketTimeout be used here as well?
There was a problem hiding this comment.
@brumi1024 Thanks for reviewing the code, I will improve it.
| KeyStoreTestUtil.cleanupSSLConfig(keystoresDir, sslConfDir); | ||
| } | ||
| TimelineConnector.DEFAULT_SOCKET_TIMEOUT = 60_000; | ||
| client.getConnector().setSocketTimeOut(60_1000); |
There was a problem hiding this comment.
Hi @slfan1989 !
Thanks for the patch!
If i see well we have a typo here, 60_1000 seems too much.
There was a problem hiding this comment.
@K0K0V0K Thanks for the review! I will improve it.
|
💔 -1 overall
This message was automatically generated. |
|
@brumi1024 @K0K0V0K Can you help review the code again? Thank you very much! |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @slfan1989 for this fix. LGTM ! |
|
💔 -1 overall
This message was automatically generated. |
|
@ayushtkn @Hexiaoqiao @brumi1024 This PR should also need to be merged before Can you help me review this PR? Thank you very much!
|
|
@brumi1024 Can you help review this PR again? Thank you very much! |
brumi1024
left a comment
There was a problem hiding this comment.
Thanks @slfan1989, the latest state LGTM. Merging to trunk.
|
@brumi1024 Thank you for reviewing the code! |
Co-authored-by: slfan1989 <slfan1989@apache.org>
Co-authored-by: slfan1989 <slfan1989@apache.org>
Co-authored-by: slfan1989 <slfan1989@apache.org>


Description of PR
JIRA: YARN-11634. [Addendum] Speed-up TestTimelineClient.
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?